Add SyncService client from EGW for embedded calling from Data API#2405
Add SyncService client from EGW for embedded calling from Data API#2405tatu-at-datastax merged 30 commits intomainfrom
Conversation
📉 Unit Test Coverage Delta vs Main Branch
|
Unit Test Coverage Report
|
📉 Integration Test Coverage Delta vs Main Branch (dse69-it)
|
Integration Test Coverage Report (dse69-it)
|
📉 Integration Test Coverage Delta vs Main Branch (hcd-it)
|
Integration Test Coverage Report (hcd-it)
|
|
Ready for some review; not sure how to test. |
| && Objects.equals( | ||
| apiException.code, | ||
| EmbeddingProviderException.Code | ||
| .EMBEDDING_GATEWAY_UNABLE_RESOLVE_AUTHENTICATION_TYPE |
There was a problem hiding this comment.
nit: no EMBEDDING_GATEWAY anymore. But I think it's not urgent, maybe update it later
There was a problem hiding this comment.
Yes -- there is SyncServiceErrorMessageMapper that needs to be rewritten to replace this exception type.
| && Objects.equals( | ||
| apiException.code, | ||
| EmbeddingProviderException.Code | ||
| .EMBEDDING_GATEWAY_UNABLE_RESOLVE_AUTHENTICATION_TYPE |
There was a problem hiding this comment.
nit: EMBEDDING_GATEWAY
| .formatted(firstError.errorId(), firstError.message()); | ||
| logger.error(msg); | ||
| throw EmbeddingProviderException.Code | ||
| .EMBEDDING_GATEWAY_UNABLE_RESOLVE_AUTHENTICATION_TYPE |
There was a problem hiding this comment.
nit: EMBEDDING_GATEWAY
| if (response.getStatus() == Response.Status.REQUEST_TIMEOUT.getStatusCode() | ||
| || response.getStatus() == Response.Status.GATEWAY_TIMEOUT.getStatusCode()) { | ||
|
|
||
| return EmbeddingProviderException.Code.EMBEDDING_GATEWAY_UNABLE_RESOLVE_AUTHENTICATION_TYPE |
There was a problem hiding this comment.
nit: EMBEDDING_GATEWAY in this file
| List<String> credNames = new ArrayList<>(); | ||
| List<Uni<Map<String, String>>> resolveUnis = new ArrayList<>(); | ||
|
|
||
| for (Map.Entry<String, String> entry : authentication.entrySet()) { |
There was a problem hiding this comment.
Great improvement! Original code only resolves one cred
| } | ||
|
|
||
| @Test | ||
| @Disabled("Requires SyncService (localhost:8084) for SHARED_SECRET credential validation") |
There was a problem hiding this comment.
Formerly non-EGW path did not call Sync Service at all; now does: would either need to Mock Sync Service, or allow configuring by-passing it or something.
But key format validation is now tested in SyncServiceCredentialResolvingProviderTest
What this PR does:
Embeds sync-service validation call code from EGW into Data API, to prepare for removal of EGW
Which issue(s) this PR fixes:
Part of #2387
Checklist